feat: add support for specifying separate credentials for authenticating with DB#510
Conversation
enocom
left a comment
There was a problem hiding this comment.
We'll want to retain the cloud-platform scope for all AlloyDB API interactions. We'll want a separate set of credentials for the metadata exchange and IAM auth that are scoped to just login. This way API interactions continue with the cloud platform scope, but any login tokens only have the login scope attached. We can talk more about this elsewhere if you like.
These credentials use `alloydb.login` scope and are specified by the `db_credentials` argument in `Connector()` and `AsyncConnector()`. The `credentials` argument continues to use the `cloud-platform` scope and is now only used for authenticating with the AlloyDB Admin API.
Done. I addressed this |
enocom
left a comment
There was a problem hiding this comment.
Some small nits. Big picture LGTM.
| request = google.auth.transport.requests.Request() | ||
| self._credentials.refresh(request) | ||
| return self._credentials.token | ||
| self._db_credentials.refresh(request) |
| self._credentials, _ = google.auth.default(scopes=scopes) | ||
| # initialize credentials for authenticating with the DB | ||
| scopes = ["https://www.googleapis.com/auth/alloydb.login"] | ||
| if db_credentials: |
There was a problem hiding this comment.
If the client has provided db_credentials, I think we don't need to change the scope. We can advice them in the docs that this should be scoped to the alloydb.login scope. Otherwise, I think we're OK to accept the credentials as is.
Only in the case below where we don't have a db_credentials do we want to ensure that we're limited to just the login scope.
| db_credentials (google.auth.credentials.Credentials): | ||
| A credentials object created from the google-auth Python library. | ||
| If not specified, the credentials used for authenticating with the | ||
| AlloyDB Admin API will also be used to authenticate with the DB. |
There was a problem hiding this comment.
We should add something like: This is only used when Auto IAM AuthN is enabled.
There was a problem hiding this comment.
Our async connector still doesn't use the metadata exchange, so here the db_credentials only get used when IAM AuthN is on. Below with the synchronous connector, the documentation you have written is perfectly fine.
e0e731d to
73b9ac3
Compare
This PR does the following:
alloydb.loginscope is used instead of thecloud-platformscope. Authentication with the AlloyDB Admin API continues to use thecloud-platformscope.db_credentialsargument passed into theConnector()andAsyncConnector(). When specified, only thedb_credentialswill be used to authenticate with the DB. If not specified, the existing behavior is preserved withcredentialsbeing used to authenticate with the DB.Fixes #501